gh-149388: Make PipeHandle.close idempotent#149518
gh-149388: Make PipeHandle.close idempotent#149518mxschmitt wants to merge 1 commit intopython:mainfrom
Conversation
mxschmitt
requested review from
1st1,
asvetlov,
kumaraditya303 and
willingc
as code owners
|
All commit authors signed the Contributor License Agreement. |
|
Closing to validate with a local CPython build before re-opening. |
|
Validated locally against a built CPython 3.16.0a0 from this branch: new regression test transitions FAIL -> PASS, full |
mxschmitt
force-pushed
the
gh-149388-pipehandle-close-idempotent
branch
from
798a844 to
37904e4
Compare
| # subsequent close() are silent no-ops. | ||
| try: | ||
| p.close() | ||
| except OSError: |
There was a problem hiding this comment.
does this still raises?
There was a problem hiding this comment.
Yes — the first close() still propagates the OSError from CloseHandle on the stale handle; the fix is that _handle is cleared first, so __del__ and any subsequent close() become silent no-ops. Reworked the test to assert exactly that with self.assertRaises(OSError).
Clear _handle before calling CloseHandle so a stale handle (closed by another code path) does not leak OSError into _ProactorBasePipeTransport._call_connection_lost.
mxschmitt
force-pushed
the
gh-149388-pipehandle-close-idempotent
branch
from
37904e4 to
d2b941f
Compare
|
Please don't force push: https://devguide.python.org/getting-started/pull-request-lifecycle/#don-t-force-push |
|
I apologize, didn't know that! Should I restore the original commit + follow-up commit? |
3 participants
Fixes #149388. Applies @kumaraditya303's suggestion from the issue thread: clear
_handlebefore callingCloseHandle, so that whenCloseHandleraises on an already-closed handle,_handleis alreadyNoneand neither a secondclose()nor__del__re-raises.The new regression test in
test_windows_utils.PipeTestsfails onmain:and passes with the patch. Full
python -m test test_asynciois green on the patched build (2524 tests, 0 failures).The end-to-end repro from the issue (
repro_v2.py) goes from 6/20 → 0/60 trials firingWinError 6. Verified against a locally built CPython 3.16.0a0 (Windows x86_64) from this branch.